[fix] Begrens limit og escape spesialtegn i søk #79
Conversation
There was a problem hiding this comment.
Fin og ryddig PR! Det er bra at hjelpefunksjonene er samlet i queryInput.ts, og testene med lookalikes (Rate 100X, CodeXA) viser at % og _ faktisk tolkes som vanlige tegn. Lint, Prettier og alle 17 API-testene besto lokalt hos meg.
Må fikses før merge:
- UUID_PATTERN i server.ts har fått en ekstra -[0-9a-f]{4}-gruppe (commit a203bd4). Den avviser nå ekte UUID-er, så userId blir alltid null og status for land slutter å virke. Ser ut som en utilsiktet endring.
Bør vurderes:
- Testene går mot resolverne i app.ts, men produksjon kjører server.ts. Endringene i server.ts er derfor ikke testet, og det er derfor UUID-feilen ikke ble fanget. Forslag: egen issue for å samle resolverne ett sted.
- offset er ikke begrenset, så offset: -1 gir en databasefeil tilbake til klienten.
Småting (valgfritt):
- Backslash-escaping er nevnt i README, men ikke testet.
- Limit-testen bruker params?.at(-2) og avhenger av rekkefølgen på parameterne. Egne enhetstester for normalizeLimit og escapeLikeSearch hadde vært mer robust.
- En kort kommentar på ESCAPE E'\\' hadde gjort den lettere å lese.
- En ekstra tom linje i testDb.ts som ikke hører til endringen.
There was a problem hiding this comment.
Ser bra ut nå! UUID-feilen er rettet og har fått tester, og offset og backslash er håndtert. Lint, Prettier, build og alle tester (23 API + 34 frontend) går gjennom lokalt. Noen småting som kan tas senere eller i en liten commit: samme describe-navn i to testfiler, JSDoc i userId.ts står over konstanten i stedet for funksjonen, og backslash-testen hardkoder 'Norway'/'Oslo' i stedet for å bruke fixtures. Vi bør også lage en issue for å samle resolverne i app.ts og server.ts.
|
Har nå fikset de siste småtingene også: ulike describe-navn, flyttet JSDoc-kommentaren og brukt fixtures i backslash-testen. Alle 23 API-tester og 34 frontend-tester består, i tillegg til lint, Prettier og build. PR-en er klar for ny review, @ellahw 😊 |
mariehd
changed the title
Hva er endret?
Begrenset antall resultater som kan hentes per GraphQL-forespørsel, forbedret håndtering av spesialtegn i søk og rettet feil som ble oppdaget under code review.
countriesogcities. Standardgrensen er fortsatt 20.%,_og\behandles som vanlige tegn i søk, i stedet for jokertegn i SQL.offset-verdier til 0 for å unngå databasefeil.userId.tsfor enklere testing.limit,offsetog søkeescaping iqueryInput.ts.server.tsogapp.ts.Refaktorering for å samle GraphQL-resolverne i én felles implementasjon tas videre i issue #74. Denne PR-en kan derfor merges uavhengig av #74.
Type endring
Test endringene
npm run lintfor å sjekke kodekvalitet med ESLint.npm run format:checkfor å sjekke formatering med Prettier.npm run buildfor å kontrollere TypeScript og produksjonsbygget.npm testfor å kjøre automatiserte tester med Vitest.npm run test:apifor å kjøre API-testene.Alle 34 frontend-tester og 23 API-tester har bestått.
API-testene dekker blant annet:
limit.offset-verdier.%,_og backslash.normalizeLimit,normalizeOffset,escapeLikeSearchog UUID-validering.Sjekkliste
Closes #73